New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge multi-source output sourcemaps #14246
Merged
nicolo-ribaudo
merged 1 commit into
babel:main
from
jridgewell:multisource-output-sourcemap
Feb 6, 2022
Merged
Merge multi-source output sourcemaps #14246
nicolo-ribaudo
merged 1 commit into
babel:main
from
jridgewell:multisource-output-sourcemap
Feb 6, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Surprisingly, Babel allows a transformer to mark the source file of a node to allow it to be sourced from any file. When this happens, the output sourcemap will contain multiple `sources`. I didn't realize this when I created babel#14209, and this `remapping` will throw an error if the output map has multiple sources. This can be fixed by using `remapping`'s graph building API (don't pass an array). This allows us to return an input map for _any_ source file, and we just need some special handling to figure out which source is our transformed file. This actually adds a new feature, allowing us to remap these multi-source outputs. Previously, the merging would silently fail and generate a blank (no `mappings`) sourcemap. That's not great. The new behavior will properly merge the maps, provided we can figure out which source is the transformed file (which should always work, I can't think of a case it wouldn't). Fixes ampproject/remapping#159.
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51177/ |
This might help with #14132 (cc @ForbiddenEra) |
nicolo-ribaudo
added
pkg: generator
PR: Bug Fix 🐛
A type of pull request used for our changelog categories
labels
Feb 6, 2022
nicolo-ribaudo
approved these changes
Feb 6, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
JLHwung
approved these changes
Feb 6, 2022
1 task
jridgewell
added a commit
to jridgewell/babel
that referenced
this pull request
Feb 7, 2022
A follow-up to babel#14246. I finally thought of a case where the sourceFileName's content is fully replaced, leading to only the injected file existing in the output sourcemap. This can happen where only one output source is created, or multiple. In the single output source case, we'd incorrectly associate the mappings through the inputMap, even though none of the content actually comes from there. In the multiple source case, we'd silently fail and output empty mappings. Both cases are now fixed, with the correct remapping being done through in all possible output cases now.
1 task
JLHwung
pushed a commit
that referenced
this pull request
Feb 8, 2022
…14247) * Support merging sourcemaps when transformed file is fully replaced A follow-up to #14246. I finally thought of a case where the sourceFileName's content is fully replaced, leading to only the injected file existing in the output sourcemap. This can happen where only one output source is created, or multiple. In the single output source case, we'd incorrectly associate the mappings through the inputMap, even though none of the content actually comes from there. In the multiple source case, we'd silently fail and output empty mappings. Both cases are now fixed, with the correct remapping being done through in all possible output cases now. * Update to remapping@2.1.0 to support source location override
github-actions
bot
added
the
outdated
A closed issue/PR that is archived due to age. Recommended to make a new issue
label
May 9, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
outdated
A closed issue/PR that is archived due to age. Recommended to make a new issue
pkg: generator
PR: Bug Fix 🐛
A type of pull request used for our changelog categories
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merge multi-source output sourcemaps
Surprisingly, Babel allows a transformer to mark the source file of
a node to allow it to be sourced from any file. When this happens, the
output sourcemap will contain multiple
sources
. I didn't realize thiswhen I created #14209, and this
remapping
will throw an error if theoutput map has multiple sources.
This can be fixed by using
remapping
's graph building API (don't passan array). This allows us to return an input map for any source file,
and we just need some special handling to figure out which source is our
transformed file.
This actually adds a new feature, allowing us to remap these
multi-source outputs. Previously, the merging would silently fail and
generate a blank (no
mappings
) sourcemap. That's not great. The newbehavior will properly merge the maps, provided we can figure out which
source is the transformed file (which should always work, I can't think
of a case it wouldn't).
See the visualization of the test case.